Repository navigation
Task/setup storybook test framework - #294
MikeNeilson wants to merge 10 commits into
Conversation
|
Please set a versioning label of either |
|
Okay, I was having that same npm ci issue in act before I opened the PR. The thing does work, just the action being wonky. Though definitely possible it's an issue with the node version or something when package.lock got updated. Will investigate at some point. |
8141903 to
12146e0
Compare
|
Okay, apparently a rather hard reset of package-lock.json is required when that start happening. |
willbreitkreutz
left a comment
willbreitkreutz
left a comment
There was a problem hiding this comment.
Couple small changes, would suggest a rebase to main to pick up the updates that have been made, likely deleting the package-lock file and re-generating it after package.json is up to date should work.
| const copiedText = mockClipboard.readText(); | ||
| expect(copiedText).toBe(args.text); | ||
| // verify green class is removed. | ||
| waitFor(() => |
There was a problem hiding this comment.
Believe we need to await the waitFor call
| @@ -0,0 +1,14 @@ | |||
| import type { Preview } from '@storybook/react-vite' | |||
|
|
|||
There was a problem hiding this comment.
I think we need to import the css here to render stuff right
There was a problem hiding this comment.
Did not appear to matter, though that was likely due to how simple the particular elements currently test are.
willbreitkreutz
left a comment
willbreitkreutz
left a comment
There was a problem hiding this comment.
Couple small changes, would suggest a rebase to main to pick up the updates that have been made, likely deleting the package-lock file and re-generating it after package.json is up to date should work.
12146e0 to
7cf9e15
Compare
|
Sticking with node 20 isn't a good idea, but the direct bump to 24 might be too much as well. Let me know if you think I should split the difference and do like 22. Apparently the package-lock.json is rather sensitive to matching the local and the action version of node. Well, or it was the particular version of node + npm I had locally. |
This branch has not been deployed
2 participants
Summary
CodeBlock: This is a simple default render so a developer can see the component. I recommend every component have at least one of these static examples. E.g. no assertions. It is also useful to provide a Story that provides behavior mocks but no assertions. This allows exploratory operations. If there are assertions in the play method Storybook will reset it in the middle of you trying to do something.
CopyButton: Asserts the behavior of the copy button given some fixed text, As well as providing the default do nothing (except visual changes) Story.
Story had to mock navigator.clipboard has that component is striped out for "security reasons" when the tests are running in the browser and just not properly available on the terminal.
See https://storybook.js for more information.
Known gotcha that was not address in this initial PR. Any component that uses state or has callbacks and needs to have it behave correctly will likely need a "wrapper" render method (goes in the story) so that storybook can render that wrapper the way it needs. Sample is available here: https://github.com/opendcs/opendcs/blob/67d6d9e655132732796b006e94dc11fb35fe5dd9/javascript/opendcs-web-ui-react/src/pages/decodes/configs/Sample/SampleRetrieve.stories.tsx#L8
(Short version, if you keep having the thing pass in the browser but fail in odd ways on the console, that's probably what's going on.)
Additional tests should most definitely be added, I will do so as time permits and specific needs arise. I will also, of course, provide guidance should anyone start having issues with how things behave.
Otherwise consider implementing code coverage and some checks to make sure coverage doesn't go down when people submit new code. OpenDCS uses SonarCloud for that, there are other options. NOTE: getting the coverage to work with vitest + storybook will be more effort, especially here as the existing vitest config is separate from the new one. Consider combining that configuration into a single vitest.config.(ts|js) file as vitest supports "projects" (see https://github.com/opendcs/opendcs/blob/67d6d9e655132732796b006e94dc11fb35fe5dd9/javascript/opendcs-web-ui-react/vite.config.ts#L89).
Checklist
patch-bump,minor-bump, ormajor-bumpRelease impact